Refactor `TemplateEmailFile` to use same sorting strategy as other models by quis · Pull Request #5841 · alphagov/notifications-admin

quis · 2026-03-16T09:45:58Z

Other models sort based on their __sort_attribute__. This commit moves the calculation of each files position in the template into the TemplateEmailFile object, so that its sorting can also work in this way.

The actual sort order is tested here:

notifications-admin/tests/app/main/views/test_template_email_files.py

Lines 26 to 45 in 0c85f9f

    
                   ( 
        
                       # Content order matches database order 
        
                       "((test_file_1.csv)) ((test_file_2.png))", 
        
                       ["test_file_1.csv", "test_file_2.png"], 
        
                   ), 
        
                   ( 
        
                       # Content order does not match database order 
        
                       "((test_file_2.png)) ((test_file_1.csv))", 
        
                       ["test_file_2.png", "test_file_1.csv"], 
        
                   ), 
        
                   ( 
        
                       # Content order does not match database order (case differs) 
        
                       "((TEST FILE 2.PNG)) ((TEST FILE 1.CSV))", 
        
                       ["test_file_2.png", "test_file_1.csv"], 
        
                   ), 
        
                   ( 
        
                       # Content order does not match database order (extra, non-file placeholders) 
        
                       "((test_file_2.png)) ((first name)) ((last name)) ((test_file_1.csv))", 
        
                       ["test_file_2.png", "test_file_1.csv"], 
        
                   ),

This is a form of dependency injection. It will be needed so the files can know their own position in the template.

Other models sort based on their `__sort_attribute__`. This commit moves the calculation of each files position in the template into the `TemplateEmailFile` object, so that its sorting can also work in this way.

…his type)

Don’t need the wrapper method on the `EmailPreviewTemplate` object now that placeholders are an `InsensitiveSet` so we can do `placeholders.index` directly

quis force-pushed the refactor-email-file-sorting branch from cd8fd91 to 09887bc Compare March 16, 2026 10:47

quis added 5 commits March 17, 2026 12:34

Pass reference to template object down to individual files

8ca2b02

This is a form of dependency injection. It will be needed so the files can know their own position in the template.

Refactor to use same sorting method as other models

139f263

Other models sort based on their `__sort_attribute__`. This commit moves the calculation of each files position in the template into the `TemplateEmailFile` object, so that its sorting can also work in this way.

Remove uneeded variable

6a55f26

Remove redundant call to InsenstiveSet (placeholders are already of t…

4aefb79

…his type)

Refactor position_in_template to look at template property directly

e89f5bd

Don’t need the wrapper method on the `EmailPreviewTemplate` object now that placeholders are an `InsensitiveSet` so we can do `placeholders.index` directly

quis force-pushed the refactor-email-file-sorting branch from 09887bc to e89f5bd Compare March 17, 2026 12:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor `TemplateEmailFile` to use same sorting strategy as other models#5841

Refactor `TemplateEmailFile` to use same sorting strategy as other models#5841
quis wants to merge 5 commits intomainfrom
refactor-email-file-sorting

quis commented Mar 16, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	(
	# Content order matches database order
	"((test_file_1.csv)) ((test_file_2.png))",
	["test_file_1.csv", "test_file_2.png"],
	),
	(
	# Content order does not match database order
	"((test_file_2.png)) ((test_file_1.csv))",
	["test_file_2.png", "test_file_1.csv"],
	),
	(
	# Content order does not match database order (case differs)
	"((TEST FILE 2.PNG)) ((TEST FILE 1.CSV))",
	["test_file_2.png", "test_file_1.csv"],
	),
	(
	# Content order does not match database order (extra, non-file placeholders)
	"((test_file_2.png)) ((first name)) ((last name)) ((test_file_1.csv))",
	["test_file_2.png", "test_file_1.csv"],
	),

Conversation

quis commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

quis commented Mar 16, 2026 •

edited

Loading